Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Angeline assignment2 #2

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Angeline assignment2 #2

wants to merge 4 commits into from

Conversation

angelinetu
Copy link
Owner

What's new in this PR

Description

pulled post data from supabase; fixed styling

Screenshots

How to review

Next steps

Relevant links

Online sources

Related PRs

CC: @ronniebeggs

Copy link
Collaborator

@ronniebeggs ronniebeggs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good so far! I left a few comments/changes. For furture reference, make sure to write more in-depth PR descriptions.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make sure you don't commit .env files! Add .env to you .gitignore file.

Comment on lines +37 to +44
const fetchPostData = async () => {
try {
const data = await getPostData();
setPostData(data);
} catch (error) {
console.error('Error fetching data:', error);
}
};
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know if you necessarily need a fetchPostData and getPostData function in the same file. Usually you'd store the getPostData function in a /supabase/post.ts queries file.

Comment on lines +85 to +99
<View style={styles.commentSection}>
{/* First Comment */}
<View style={styles.comment}>
<ProfilePlaceholder style={styles.commentIcon} />
<View style={styles.commentBody}>
<View style={styles.commentHeader}>
<Text style={styles.commentName}>philip_ye</Text>
<Text style={styles.commentDate}>September 20</Text>
</View>
<Text style={styles.commentText}>
This organization is doing amazing work tackling the complex
root causes of the issue.
</Text>
</View>
</View>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that there are so many divs for each div, might be helpful to create a separate Comment component.

There's also a comments table that you can pull this data from!

borderTopColor: '#ccc',
paddingTop: 10,
marginTop: 20,
paddingRight: 10,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Avoid asymmetric spacing attributes. See if you can achieve this styling using flexbox properties.

},
postDate: {
color: 'gray',
paddingLeft: 120,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should be able to create an offset like this using justifyContent and alignItems attributes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants